-
Notifications
You must be signed in to change notification settings - Fork 277
[WC] Scale Estimation transpose_a support #3839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
[WC] Scale Estimation transpose_a support #3839
Conversation
| act_ch_axis = self._backend_entity.get_activation_channel_axis( | ||
| wp.node_with_weight, activation_port_id, act_shape | ||
| ) | ||
| act_ch_axis %= len(act_shape) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For which case it's need?
All tests still pass if remove the line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outdated
|
|
||
| weight = self._backend_entity.get_weight(wp.node_with_weight, weight_port_id, model, graph) | ||
|
|
||
| activation_port_id = self._backend_entity.get_activation_port_id(wp.node_with_weight, graph) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like copypast from awq.py.
Please think about to refactor it into a shared function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WeightCompressionAlgoBackend.get_activation_channel_axis_and_shape is instroduced, please check
|
|
||
| matmul = opset.matmul(input_1, weight_data, transpose_a=False, transpose_b=False, name="MoE_MatMul") | ||
| if tranpsose_a: | ||
| transpose = opset.transpose(input_1, (0, 2, 1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check, looks like it never runs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch,
It is, because of this skip https://github.com/daniil-lyakhov/nncf/blob/dl/sa_transpose_a/tests/openvino/native/quantization/test_weights_compression.py#L2369
I fixed the test and asked @anzr299 to remove the skip. Ticket 179366
Changes
transpose_aattribute for ONNX/OpenVINO backends in the Scale Estimation algorithmactivations_to_wc_statisticsmoved from the Scale Estimation algorithm to the GPTQReason for changes
Related tickets
173277
Tests
tests/cross_fw/test_templates/template_test_weights_compression.py::test_scale_estimation updated with the
transpose_acases